-
Notifications
You must be signed in to change notification settings - Fork 2
Feature: add new formatters for attendees #481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
d70b11e to
2e6a6c5
Compare
|
The branch was rebased and now it is ready for review. |
b26ad08 to
7c660e5
Compare
|
@smarcet this will bet Unit Tested with the change introduced with PR 479. Please can you review this PR. |
caseylocker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical Issues (P1)
These issues will throw PHP Error (not Exception) at runtime, which is not caught by the existing catch (\Exception $ex) blocks. This will abort transactions and cause data loss.
- SummitAttendeeAuditLogFormatter.php:20
Problem: SummitAttendee has getSurname(), not getLastName()
// Current (broken):
$name = trim(($subject->getFirstName() ?? '') . ' ' . ($subject->getLastName() ?? '')) ?: 'Unknown Attendee';
// Fix:
$name = trim(($subject->getFirstName() ?? '') . ' ' . ($subject->getSurname() ?? '')) ?: 'Unknown Attendee';
- SummitAttendeeBadgeAuditLogFormatter.php:19-21
Problem: Two issues:
- SummitAttendeeBadge doesn't have getOwner() — must go through getTicket()
- Owner is SummitAttendee which has getSurname(), not getLastName()
// Current (broken):
$id = $subject->getId() ?? 'unknown';
$owner = $subject->getOwner();
$owner_name = $owner ? trim(($owner->getFirstName() ?? '') . ' ' . ($owner->getLastName() ?? '')) : 'Unknown Owner';
// Fix:
$id = $subject->getId() ?? 'unknown';
$ticket = $subject->getTicket();
$owner = $ticket?->getOwner();
$owner_name = $owner ? trim(($owner->getFirstName() ?? '') . ' ' . ($owner->getSurname() ?? '')) : 'Unknown Owner';
- SummitAttendeeTicketAuditLogFormatter.php:21
Problem: getOwner() returns SummitAttendee which has getSurname(), not getLastName()
// Current (broken):
$owner_name = $owner ? trim(($owner->getFirstName() ?? '') . ' ' . ($owner->getLastName() ?? '')) : 'Unknown Owner';
// Fix:
$owner_name = $owner ? trim(($owner->getFirstName() ?? '') . ' ' . ($owner->getSurname() ?? '')) : 'Unknown Owner';
- SummitAttendeeNoteAuditLogFormatter.php:21
Problem: getOwner() returns SummitAttendee which has getSurname(), not getLastName()
// Current (broken):
$owner_name = $owner ? trim(($owner->getFirstName() ?? '') . ' ' . ($owner->getLastName() ?? '')) : 'Unknown Owner';
// Fix:
$owner_name = $owner ? trim(($owner->getFirstName() ?? '') . ' ' . ($owner->getSurname() ?? '')) : 'Unknown Owner';
Minor Housekeeping:
You're also missing Copyright headers on the new files. Check the existing formatters for examples:
/**
- Copyright 2025 OpenStack Foundation
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
- http://www.apache.org/licenses/LICENSE-2.0
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
**/
7c660e5 to
5fed732
Compare
…octrine event listener
Task:
Ref: https://app.clickup.com/t/86b81xtwx